-
Notifications
You must be signed in to change notification settings - Fork 29
Only display the input fields that are configured in the scope #631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a configuration option that restricts the display of About input fields in the editors by passing a set of visible fields down through the view model and UI components. Key changes include:
- Replacing usages of AboutInputField with AboutEditorField in the editor components and tests.
- Updating the view model and profile mapping to respect the configured visibility flags.
- Adding a new about fields picker in the demo app to allow dynamic field configuration.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
AboutEditorViewModelTest.kt | Updated tests to use the new visibility configuration and field type. |
AboutEditorTest.kt | Modified tests to verify the UI state correctly reflects visible fields. |
QuickEditorScopeOption.kt | Added a getter to expose aboutFields based on the scope configuration. |
ProfessionalSection.kt, PersonalSection.kt, AboutSection.kt | Wrapped field rendering with conditional visibility checks and updated field types. |
AboutFields.kt | Modified data classes and update functions to use AboutEditorField with a new visible flag. |
AboutEditorViewModel.kt | Changed the view model to map the profile using configured visible fields and updated API types. |
AboutEditorEvent.kt, AboutEditor.kt | Adjusted event and editor definitions to use AboutEditorField. |
AboutFieldsBottomSheet.kt | Adapted the bottom sheet component to work with the field configuration. |
AvatarUpdateTab.kt | Integrated an about fields picker and updated scope configuration to use the chosen fields. |
demo-app/build.gradle.kts | Added a dependency on composable core components. |
...ar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/abouteditor/AboutEditorViewModel.kt
Outdated
Show resolved
Hide resolved
📲 You can test the changes from this Pull Request in Gravatar Demo by scanning the QR code below to install the corresponding build.
|
@etoledom I've marked this as a Draft. I'm not 100% happy with how the state is modeled in the |
@etoledom This is now ready to review. I did modify the model for the AboutEditor state in 94bcfd7. Mainly, I've decided to keep about fields as a |
52b2e27
to
a80dc53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for displaying only configured About input fields in the editor based on scope settings.
- Passes
aboutFields
from scope config intoAboutEditorViewModel
to control visibility. - Refactors state and UI to use
AboutEditorField
sets instead of fixedAboutFields
classes. - Introduces a bottom sheet in the demo app for selecting visible About input fields.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
AboutEditorViewModelTest.kt | Updated tests to use new AboutEditorField API |
AboutEditorViewModel.kt | Accepts visibleAboutFields and maps profile fields accordingly |
AboutSection.kt | Refactored UI to render dynamic field sets |
AboutInputField.kt | Added ordering and category helpers for fields |
AboutFieldsBottomSheet.kt | New UI for picking visible About fields in demo app |
AvatarUpdateTab.kt | Integrated bottom sheet and bound selected fields in demo app |
Comments suppressed due to low confidence (3)
gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/ui/abouteditor/AboutEditorViewModelTest.kt:258
- You’ve added a test for personal-only visibility—consider adding a complementary test for the professional-only case to ensure that section toggling works as expected.
fun `given only personal about section visible when profile fetched then uiState is updated`() = runTest {
demo-app/src/main/java/com/gravatar/demoapp/ui/components/AboutFieldsBottomSheet.kt:44
- [nitpick] The parameter name
aboutFields
in this sheet conflicts with the editor’saboutFields
type (AboutEditorField
). Consider renaming toselectedFields
or similar for clarity.
internal fun AboutFieldsBottomSheet( aboutFields: Set<AboutInputField>,
gravatar-quickeditor/src/test/java/com/gravatar/quickeditor/ui/abouteditor/AboutEditorViewModelTest.kt:8
- Tests reference
AboutEditorField
, but there's no import; addimport com.gravatar.quickeditor.ui.abouteditor.AboutEditorField
to avoid compilation errors.
import com.gravatar.quickeditor.ui.editor.AboutInputField
...quickeditor/src/main/java/com/gravatar/quickeditor/ui/abouteditor/components/AboutSection.kt
Show resolved
Hide resolved
...ar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/abouteditor/AboutEditorViewModel.kt
Show resolved
Hide resolved
gravatar-quickeditor/src/main/java/com/gravatar/quickeditor/ui/editor/AboutInputField.kt
Show resolved
Hide resolved
b4cad07
to
eba852e
Compare
a80dc53
to
b451906
Compare
There's a crash caused by rememberSaveable { mutableStateOf(AboutInputField.all) } The default saver doesn't support non-primitive types.
b451906
to
004a452
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great 🎉
One small detail comparing with Figma:
It seems that when there's only one section present, the section title is not being shown:
Figma | Implemented |
---|---|
![]() |
![]() |
This is how it was implemented on iOS also.
If there are fields from only one section, there's no section title. But when there are at least 1 field from each section, the section titles are being shown.
Same section | Different sections |
---|---|
![]() |
![]() |
Ohh I missed that detail, thanks. |
@etoledom Fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! Thank you for the update 🙏
a461f4d
into
feature/quick_editor_about_info
Description
This PR allows displaying only certain AboutInputFields in the editors per the scope config.
The public API was already done. The changes made are:
aboutFields
from the scope config to theAboutEditorViewModel
and set the visibility in the state.aboutFields.mp4
Testing Steps
About
orAvatarAndAbout
scopeAbout fields